fix: update apply_patch format instructions to be consistent and include required space #531
Conversation
| {isGpt5 && <>Prefer the smallest set of changes needed to satisfy the task. Avoid reformatting unrelated code; preserve existing style and public APIs unless the task requires changes. When practical, complete all edits for a file within a single message.<br /></>} | ||
| {isGpt5 && <> | ||
| Never use {ToolName.ApplyPatch} for creating files.<br /> | ||
| Prefer the smallest set of changes needed to satisfy the task. Avoid reformatting unrelated code; preserve existing style and public APIs unless the task requires changes. When practical, complete all edits for a file within a single message.<br /></>} |
There was a problem hiding this comment.
Also, do we need these instructions? Would they limit GPT-5 at all and have GPT-5 focus on only making small simple changes?
Prefer the smallest set of changes needed to satisfy the task. Avoid reformatting unrelated code; preserve existing style and public APIs unless the task requires changes. When practical, complete all edits for a file within a single message.<br />
There was a problem hiding this comment.
Not sure what the origin of this line is. cc @bhavyaus
| @@ -433,7 +441,9 @@ class ApplyPatchInstructions extends PromptElement<DefaultAgentPromptProps> { | |||
| const isGpt5 = this.props.modelFamily === 'gpt-5'; | |||
| return <Tag name='applyPatchInstructions'> | |||
| To edit files in the workspace, use the {ToolName.ApplyPatch} tool. If you have issues with it, you should first try to fix your patch and continue using {ToolName.ApplyPatch}. If you are stuck, you can fall back on the {ToolName.EditFile} tool. But {ToolName.ApplyPatch} is much faster and is the preferred tool.<br /> | |||
| {isGpt5 && <>Prefer the smallest set of changes needed to satisfy the task. Avoid reformatting unrelated code; preserve existing style and public APIs unless the task requires changes. When practical, complete all edits for a file within a single message.<br /></>} | |||
| {isGpt5 && <> | |||
| Never use {ToolName.ApplyPatch} for creating files.<br /> | |||
There was a problem hiding this comment.
Creating a file with apply patch is fine, we should handle it
If we don't, that is a bug we should fix
There was a problem hiding this comment.
I can do some additional testing today as I'm working on things and report back regarding apply_patch failing on creating a file.
There was a problem hiding this comment.
We don't explicitly instruct it to use Add File afaik in our instructions, but GPT is fine-tuned on apply_patch so it might include it nevertheless
| <br /> | ||
| Prefix each line of old code with exactly one minus (-) before its original first character; keep the original indentation after it.<br /> |
There was a problem hiding this comment.
These prefix instructions don't appear in Codex's apply patch instructions, so I would tend to say we should not include them here unless we have strong reason to believe they improve behavior https://github.com/openai/codex/blob/main/codex-rs/apply-patch/apply_patch_tool_instructions.md
There was a problem hiding this comment.
Good point, I can look at Codex's code first and see where this is a problem. Also I'll try testing this out further and produce smaller set of sample data to reproduce the issue.
There was a problem hiding this comment.
I made a repo with contrived examples, capturing conversation, example file before conversation, results, and all logs.
https://github.com/agreaves-ms/vscode-copilot-chat-example-issues
The one example I have in there right now is specifically around what these instructions were meant to fix.
@connor4312 do you think it makes sense to share this and address these instruction changes in Codex's repo first?
I don't believe it's possible to work around this issue in apply_patch's logic as it would always see something like - list item differently than - list item.
Captured screenshot of the problem:
### 🔐 SSL and Kubernetes Integration (SSE-Only) should be ### 🔐 SSL and Kubernetes Integration (SSE-Only)
ApplyPatchFormatInstructions
[context_before]and[context_after]-before original old code lines+after new code linesApplyPatchInstructions
Details
The apply_patch instructions and description would sometimes confuse the models and produce patches that where either missing an additional space infront of unchanging context lines, or it would consider
-characters from the original text as old code lines.